Skip to content

docs: clean up headerdocs#2829

Merged
armcknight merged 5 commits intomainfrom
armcknight/docs/standardize-format-defaults-and-monospaced-elements
Mar 23, 2023
Merged

docs: clean up headerdocs#2829
armcknight merged 5 commits intomainfrom
armcknight/docs/standardize-format-defaults-and-monospaced-elements

Conversation

@armcknight
Copy link
Member

@armcknight armcknight commented Mar 23, 2023 •
edited
Loading

After making some of the changes in #2804 , I noticed some other irregularities in nearby headerdocs.

  • set aside all declarations of default values in separate @note entries
  • add @c to sentry and builtin declared symbols in headerdocs and other tokens that are better viewed in monospace,
    • like values that can be supplied to a documented method: 0 is easy to mistake as O in variable-width fonts, but 0 and O are better because of the slash through the monospace 0.
    • Also, tokens containing underscores would cause all subsequent text to be rendered as italic due to the markdown rendering, which doesn't happen if it is preceded by a @c
  • some light editing of typos, whitespace and extra newlines; reworded some phrasing to be more clear
  • removed some, but probably not all, superfluous headerdocs. things like a doc over an init method for class Foo that states @returns Foo (example). I'd rather just save that line and vertical space.
  • standardize on @c for monospaced elements in headerdocs. This gives it treatment in the source file and in doc popovers.
    • while backticks do make it monospaced in source files, it doesn't in doc popovers
      • before:
        Screenshot 2023-03-22 at 5 41 00 PM
      • after:
        Screenshot 2023-03-22 at 5 41 23 PM
    • <code></code doesn't appear to do anything:
      • before:
        Screenshot 2023-03-22 at 5 44 19 PM
        Screenshot 2023-03-22 at 5 44 22 PM
      • after:
        Screenshot 2023-03-22 at 5 44 32 PM
        Screenshot 2023-03-22 at 5 44 39 PM

#skip-changelog

@armcknight
standardize on @c for monospaced elements in headerdocs; set aside al…
03f9d56 
…l declarations of default values in separate @notes; add @c to sentry and builtin declared symbols in headerdocs; some light editing of typos, whitespace and extra newlines
@armcknight
change Http -> HTTP
b288659
@armcknight
add more @todos
0a04dda
@armcknight
Copy link
Member Author

armcknight commented Mar 23, 2023 •
edited
Loading

Hmm, I just noticed the Build Documentation feature in Xcode, which @c doesn't play well with. And while I see some cross-reference links created for tokens surrounded by double-backticks, that doesn't work perfectly. Since we didn't really use that feature, I think we should stick with @c for now.

@armcknight armcknight marked this pull request as draft March 23, 2023 01:31
@armcknight armcknight marked this pull request as ready for review March 23, 2023 01:39
@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1193.50 ms 1229.90 ms 36.40 ms
Size 20.76 KiB 425.81 KiB 405.04 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8f397a7 1196.55 ms 1226.82 ms 30.27 ms
8f397a7 1252.37 ms 1274.80 ms 22.43 ms
7bc3c0d 1212.35 ms 1228.94 ms 16.59 ms
ce4cfaf 1203.61 ms 1218.86 ms 15.25 ms
06548c0 1226.71 ms 1252.37 ms 25.66 ms
4259afd 1222.12 ms 1249.74 ms 27.62 ms
7fb7afb 1235.00 ms 1256.81 ms 21.81 ms
369222e 1232.14 ms 1258.90 ms 26.76 ms
c9724f9 1199.38 ms 1229.54 ms 30.16 ms
28333b6 1247.29 ms 1262.51 ms 15.22 ms

App size

Revision Plain With Sentry Diff
8f397a7 20.76 KiB 420.55 KiB 399.79 KiB
8f397a7 20.76 KiB 420.55 KiB 399.79 KiB
7bc3c0d 20.76 KiB 427.35 KiB 406.59 KiB
ce4cfaf 20.76 KiB 423.19 KiB 402.43 KiB
06548c0 20.76 KiB 427.36 KiB 406.59 KiB
4259afd 20.76 KiB 419.70 KiB 398.94 KiB
7fb7afb 20.76 KiB 419.69 KiB 398.94 KiB
369222e 20.76 KiB 419.67 KiB 398.91 KiB
c9724f9 20.76 KiB 427.66 KiB 406.90 KiB
28333b6 20.76 KiB 424.69 KiB 403.93 KiB

Previous results on branch: armcknight/docs/standardize-format-defaults-and-monospaced-elements

Startup times

Revision Plain With Sentry Diff
bda1d49 1249.94 ms 1268.84 ms 18.90 ms

App size

Revision Plain With Sentry Diff
bda1d49 20.76 KiB 425.81 KiB 405.04 KiB

@codecov
Copy link

codecov bot commented Mar 23, 2023 •
edited
Loading

Codecov Report

Merging #2829 (e84f91a) into main (9d56232) will increase coverage by 0.03%.
The diff coverage is n/a.

❗ Current head e84f91a differs from pull request most recent head 83cd1f5. Consider uploading reports for the commit 83cd1f5 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2829      +/-   ##
==========================================
+ Coverage   81.30%   81.33%   +0.03%     
==========================================
  Files         258      258              
  Lines       24131    24150      +19     
  Branches    10706    10719      +13     
==========================================
+ Hits        19619    19642      +23     
+ Misses       4014     4011       -3     
+ Partials      498      497       -1
Impacted Files Coverage Δ
Sources/Sentry/SentryAppStateManager.m 100.00% <ø> (ø)
Sources/Sentry/SentryCrashReportConverter.m 93.93% <ø> (ø)
Sources/Sentry/SentryDevice.mm 53.09% <ø> (ø)
Sources/Sentry/SentryHttpTransport.m 97.73% <ø> (ø)
Sources/Sentry/SentryHub.m 95.98% <ø> (ø)
Sources/Sentry/SentryMetricKitIntegration.m 99.60% <ø> (ø)
Sources/Sentry/SentryNSURLSessionTaskSearch.m 100.00% <ø> (ø)
Sources/Sentry/SentryOptions.m 96.99% <ø> (ø)
Sources/Sentry/SentryReachability.m 81.81% <ø> (ø)
Sources/Sentry/SentrySDK.m 86.20% <ø> (ø)
... and 9 more

... and 4 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d56232...83cd1f5. Read the comment docs.

@philipphofmann
Copy link
Member

philipphofmann commented Mar 23, 2023

Hmm, I just noticed the Build Documentation feature in Xcode, which @c doesn't play well with. And while I see some cross-reference links created for tokens surrounded by double-backticks, that doesn't work perfectly. Since we didn't really use that feature, I think we should stick with @c for now.

We can revisit this once we start publishing code docs.

philipphofmann
philipphofmann approved these changes Mar 23, 2023
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for doing this 👏 💯 🥇

Please merge this quickly to avoid conflicts. LGTM 🌟

@armcknight armcknight merged commit fb53d97 into main Mar 23, 2023
@armcknight armcknight deleted the armcknight/docs/standardize-format-defaults-and-monospaced-elements branch March 23, 2023 21:25
@armcknight
Copy link
Member Author

armcknight commented Mar 23, 2023

@philipphofmann This was the first time I encountered the high risk file check, note that I added the new sha to its output to tighten up the round trip in that workflow a bit: https://github.com/getsentry/sentry-cocoa/pull/2829/files#diff-9219d0ee596a5725e4e618eb75d7fb9497df6ad02b984e46b857855d03161922R13

@philipphofmann philipphofmann mentioned this pull request Mar 24, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@philipphofmann philipphofmann philipphofmann approved these changes

@brustolin brustolin Awaiting requested review from brustolin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@armcknight @philipphofmann